fix(security): filepath.IsLocal sanitisers for CodeQL path/command-injection#44
Merged
fix(security): filepath.IsLocal sanitisers for CodeQL path/command-injection#44
Conversation
…d-injection CodeQL flagged 8 alerts (1 critical command-injection + 7 high path-injection) in notes and project handlers. The underlying code is already safe — ValidateKey/IsValidSlug reject path-dangerous input — but CodeQL's static analysis doesn't recognise our regex/segment sanitisers, so the warnings persisted. Adds filepath.IsLocal() checks at the tainted sinks: - internal/notes/notes.go::resolvePath (covers Read/Write/Delete/List) - internal/notes/history.go::autoCommit + History - internal/api/project.go::projectMiddleware filepath.IsLocal is the CodeQL-recognised sanitiser for Go path injection; it's stricter than our existing checks so this is pure belt-and-braces. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
aksOps
added a commit
that referenced
this pull request
Apr 23, 2026
CodeQL alert #33 (Critical): exec.Command("git", ...) in runGit received user-controlled strings (commit message bytes from author / subject, and paths from note keys). Although exec.Command uses argv (no shell) which makes the standard shell-injection vector inert, git itself has flag-based attack surface (e.g. `--upload-pack=cmd`, `-c core.sshCommand=...`) that CodeQL is right to flag. Defense in depth: 1. `runGit` now enforces: - notesDir must be non-empty and must not start with "-" (prevents it being parsed as a git top-level flag). - args[0] must be in a closed allow-list of subcommands (init / config / add / log). Commit is deliberately routed elsewhere (see #2). - Any arg after a literal "--" must satisfy filepath.IsLocal — this continues the sanitiser from PR #44 and is what CodeQL recognises. 2. `gitCommit` is a new helper that runs `git commit --no-gpg-sign -F -` and pipes the message via stdin. The message bytes therefore never appear in argv, cutting the only remaining taint flow from user input to exec.Command args. 3. autoCommit's commit call switched from runGit(..., "commit", "-m", msg) to gitCommit(notesDir, msg). Tested locally: 104/104 tests passing in internal/notes/. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes 8 CodeQL alerts (1 critical command-injection + 7 high path-injection). Underlying code was already safe via regex/segment validation; CodeQL just doesn't recognise those as sanitisers. Added
filepath.IsLocalchecks at the tainted sinks — CodeQL-recognised, stricter than our existing checks, so this is pure belt-and-braces.Sites:
internal/notes/notes.go::resolvePath(covers Read/Write/Delete/List)internal/notes/history.go::autoCommit+Historyinternal/api/project.go::projectMiddlewareTests pass locally.